Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create v1 HPA types #20501

Merged
merged 9 commits into from
Feb 18, 2016
Merged

Create v1 HPA types #20501

merged 9 commits into from
Feb 18, 2016

Conversation

piosz
Copy link
Member

@piosz piosz commented Feb 2, 2016

This PR creates a new api group autoscaling, moves HPA types there with support for extensions/v1beta1.

This is still in progress (unit&integration tests should be fixed) although I'm sending for early feedback.

In a follow up PR I'll make changes in HPA types described in #18527 (comment) and #18528 (comment). I don't want to complicate this PR even more. Hopefully it's ok to make a breaking changes shortly after introducing this version.

@piosz
Copy link
Member Author

piosz commented Feb 2, 2016

cc @bgrant0607 @davidopp @deads2k @fgrzadkowski @liggitt @mwielgus @ncdc @nikhiljindal @smarterclayton
@kubernetes/goog-csi @kubernetes/autoscaling

@piosz
Copy link
Member Author

piosz commented Feb 2, 2016

This PR basically implements what @smarterclayton proposed in #18528 (comment)

@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 2, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2016
@lavalamp lavalamp assigned nikhiljindal and unassigned lavalamp Feb 2, 2016
@lavalamp
Copy link
Member

lavalamp commented Feb 2, 2016

@nikhiljindal can you review this one?

@davidopp
Copy link
Member

davidopp commented Feb 2, 2016

cc/ @madhusudancs

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2016
@piosz
Copy link
Member Author

piosz commented Feb 3, 2016

FYI @madhusudancs this PR creates autoscaling group.

nikhiljindal
nikhiljindal reviewed Feb 3, 2016
View reviewed changes
pkg/client/unversioned/autoscaling.go
@@ -0,0 +1,82 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to change to 2016 everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is checked by verify-boilerplate script. We should update the script i change everywhere I guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only change in new files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@nikhiljindal
Copy link
Contributor

This PR is huge.
Would be great if you can break it down. The last 2 commits can surely go in a separate PR.

Re: making a breaking change to v1: yes its fine (thats what we did when we moved from v1beta3 to v1, we first copied types as is and then made changes). We just need to make sure that there is no release in between.

@piosz
Copy link
Member Author

piosz commented Feb 3, 2016

@nikhiljindal I tried to break the PR down unfortunately I got into some issues and I didn't have enough time to do it. Anyway the PR is split across a bunch of commits so it should be reviewable.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2016
@madhusudancs
Copy link
Contributor

@piosz What exactly are you going to implement in this - #18528 (comment)? Just want to draw your attention to PR #19802, commit madhusudancs@aebc016 which partly implements what is described in that comment. Specifically it implements what is described here - #18528 (comment) and waiting on autoscaling API group to be created. However, it doesn't implement the comment you linked to fully. We need to collaborate on this to avoid effort duplication.

Btw, these PR is blocking a bunch of other PRs that are already LGTM'ed.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2016
@piosz piosz added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. labels Feb 18, 2016
@piosz
Copy link
Member Author

piosz commented Feb 18, 2016

Also in c02e966 I fixed unit tests. @lavalamp PTAL

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 18, 2016
@piosz piosz added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 18, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 18, 2016

GCE e2e test build/test passed for commit c02e966.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Feb 18, 2016

GCE e2e test build/test passed for commit c02e966.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Feb 18, 2016
Merge pull request #20501 from piosz/hpa-ga
17325ef 
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit 17325ef into kubernetes:master Feb 18, 2016
@madhusudancs madhusudancs mentioned this pull request Feb 19, 2016
Closed
@fgrzadkowski fgrzadkowski mentioned this pull request Feb 19, 2016
Closed
erictune added a commit to erictune/kubernetes that referenced this pull request Feb 19, 2016
@erictune
Created batch API group and copied Job there
98a801a 
Modeled after on first commit (2fbc5bb) of piosz:hpa-ga (kubernetes#20501).
@deads2k deads2k mentioned this pull request Feb 19, 2016
Closed
@piosz piosz mentioned this pull request Feb 22, 2016
Merged
@piosz piosz deleted the hpa-ga branch February 22, 2016 20:40
@deads2k deads2k mentioned this pull request Mar 4, 2016
Closed
@piosz piosz added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 16, 2016
@ixdy ixdy mentioned this pull request May 27, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lavalamp lavalamp

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet